Skip to content
This repository has been archived by the owner on Oct 30, 2024. It is now read-only.

Target Node 12 #47

Closed
wants to merge 1 commit into from
Closed

Target Node 12 #47

wants to merge 1 commit into from

Conversation

AlicanC
Copy link

@AlicanC AlicanC commented May 29, 2021

Targets Node 12 using @tsconfig/node12. I initially tried Node 14 (which gave better build output), but then dropped to 12 after being concerned for losing compatibility. Before the change it was 8-10-ish, but both versions reached EOL.

@holgerd77
Copy link
Member

Hi @AlicanC,
thanks for the PRs opened. This is our central configuration repository for various libraries of the monorepo and we are very conservative on changes, since changes here can relatively easily break things or at least significantly disrupt the development flow.

If you see some issues here I would ask you to first open an issue in favor of directly open PRs with assumed fixes and start on a discussion there, optimally with one very targeted item at a time. Agreements may take some time, since these things need some broader consensus within the team.

Thanks! 😄

@AlicanC
Copy link
Author

AlicanC commented May 31, 2021

Hi @AlicanC,
thanks for the PRs opened. This is our central configuration repository for various libraries of the monorepo and we are very conservative on changes, since changes here can relatively easily break things or at least significantly disrupt the development flow.

If you see some issues here I would ask you to first open an issue in favor of directly open PRs with assumed fixes and start on a discussion there, optimally with one very targeted item at a time. Agreements may take some time, since these things need some broader consensus within the team.

Thanks! 😄

Understandable 👍

I want to learn how Ethereum works internally and I'm a JS dev so I started studying EthereumJS. To comfortably study a project I need to make certain updates so it plays fine with my dev environment. That's mainly what I'm doing here. While I'll continue doing that on my forks, I'll stop spamming PRs here 😄

One problem is (besides the fact that I completely forgot to check open issues before sending the PRs), there's already a similar issue, #9, and it's from 2018. That pace, how should I put it, is a little bit slow for me 😄

What kind of an issue would take your attention? I'm interested in removing outdated dependencies, upgrading dev dependencies, adding a few lint rules, converting shell scripts to JS, improving monorepo DX, improving VSCode DX and such.

I believe these changes would make EthereumJS more accessible to new devs.

@holgerd77
Copy link
Member

@AlicanC ah yes, this issue can be closed, sorry. We have done some update on differentiating the build targets for Node.js and the browser in #28. But to give you some context: this discussion to actually do that has taken months within the whole team, doing changes here is super-sensitive since the libraries we are developing are used on a large variety of platforms, so we just can't jump these kind of versions on the sideline (even if we sometimes would like to do ourselves 😋 ).

I guess the config library is generally not so well suited for contributions. You can instead e.g. target NYC for test coverage from this issue ethereumjs/ethereumjs-monorepo#832 if you want and see how this goes? That would be pretty useful! 😄

@AlicanC AlicanC mentioned this pull request Jun 4, 2021
@AlicanC
Copy link
Author

AlicanC commented Jun 4, 2021

Gothca 👍 I've decided to focus on the monorepo and leave config alone.

@AlicanC AlicanC closed this Jun 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants